Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add check for ZFS #1349

Closed
wants to merge 57 commits into from
Closed

Add check for ZFS #1349

wants to merge 57 commits into from

Conversation

mboylevt
Copy link

@mboylevt mboylevt commented Feb 5, 2015

I've added some basic checks for the zfs filesystem. These checks give insights into space used/available, compression/deduplication ratio, and basic health checks.

@remh
Copy link
Contributor

remh commented Feb 5, 2015

Thanks a lot @mboylevt ! This looks like an awesome pull request!

We are currently in the process of releasing our 5.2.0 agent, hence our master branch is frozen for now.

We will review your Pull request when we will be preparing our 5.3.0 release.
Thanks again!

@remh
Copy link
Contributor

remh commented Feb 5, 2015

Also one first nitpick, could you squash these commits into a single one please ?
Thanks again!

@@ -0,0 +1,232 @@
#!/usr/bin/python
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need this line

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

@mboylevt
Copy link
Author

mboylevt commented Mar 3, 2015

@remh at this point, I'm not sure how to squash my original commits w/o squashing the merged in commits from your master as well. Would you prefer that I simply re-open this pull request?

@remh remh modified the milestones: To be determined, 5.3.0 Mar 16, 2015
@mboylevt
Copy link
Author

Any update on getting this merged?

@remh
Copy link
Contributor

remh commented May 18, 2015

Hi @mboylevt sorry for the long delay.
Unfortunately we won't merge this PR.

The mains are that:

  • We are trying to avoid the use of subprocess more and more in the agent
  • This pull requests requires root access to work and there is currently no nice way for a user to run the agent as root (and we don't want to recommend doing it).

In the same time, you can definitely use it as a custom check without having this merged, so it shouldn't block you.

Thanks a lot for your time and let us know if you have more questions.

@remh remh closed this May 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants